common: remove cgv1 for podman6 #417
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ A new PR has been created in buildah to vendor these changes: containers/buildah#6449 |
lsm5
changed the title
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
lsm5
force-pushed
the
podman6-no-cgv1-new
branch
from
e82ef0f to
c69aaf4
Compare
|
Packit jobs failed. @containers/packit-build please check. |
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
lsm5
force-pushed
the
podman6-no-cgv1-new
branch
from
1c21882 to
453e6e8
Compare
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
lsm5
force-pushed
the
podman6-no-cgv1-new
branch
from
9002044 to
7cf077c
Compare
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
lsm5
force-pushed
the
podman6-no-cgv1-new
branch
from
7cf077c to
c30076c
Compare
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 24, 2025
|
@Luap99 @mtrmac PTAL. Smaller commits so should be hopefully easier to review. I haven't removed any of the IsCgroup2UnifiedMode calls where there were error checks. Only assumed the bool to be true there, and removed the cgv1 parts. Unless there are any critical blockers like some change being obviously wrong or some cgv1 logic still present and open to being called, I'd prefer to leave any cleanups / simplifications for followup. |
|
should obsolete #387 |
lsm5
changed the title
Replace with cgroups.IsCgroup2UnifiedMode. Signed-off-by: Lokesh Mandvekar <[email protected]>
And all the cgroup v1 logic that relied on the cgroup2 bool Signed-off-by: Lokesh Mandvekar <[email protected]>
- exclude param from getAvailableControllers - cgroupV1GetAllSubsystems - CgroupControl.createGroupDirectory - readAcct - readAcctList Signed-off-by: Lokesh Mandvekar <[email protected]>
Also inline some struct element values with constant values Signed-off-by: Lokesh Mandvekar <[email protected]>
Signed-off-by: Lokesh Mandvekar <[email protected]>
Signed-off-by: Lokesh Mandvekar <[email protected]>
Signed-off-by: Lokesh Mandvekar <[email protected]>
Signed-off-by: Lokesh Mandvekar <[email protected]>
I can only see it being called in libpod/oci_conmon_linux.go that too when CgroupManager is not systemd, so it should be safe to remove. Signed-off-by: Lokesh Mandvekar <[email protected]>
Signed-off-by: Lokesh Mandvekar <[email protected]>
Signed-off-by: Lokesh Mandvekar <[email protected]>
lsm5
force-pushed
the
podman6-no-cgv1-new
branch
from
df3ce3a to
26e4421
Compare
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 25, 2025
Signed-off-by: Lokesh Mandvekar <[email protected]>
podmanbot
pushed a commit
to podmanbot/buildah
that referenced
this pull request
Oct 25, 2025
lsm5
changed the title
lsm5
changed the title
|
@mheon PTAL. |
mheon
reviewed
Oct 29, 2025
|
LGTM |
mtrmac
approved these changes
Oct 30, 2025
| } | ||
| return subsystems, nil | ||
| // AvailableControllers get string:bool map of all the available controllers. | ||
| func AvailableControllers(exclude map[string]controllerHandler) ([]string, error) { |
There was a problem hiding this comment.
In “Remove unused parameters and functions post CgroupControl cleanup”: Drop the unused exclude parameter as well?
(And, separately, getAvailableControllers could be inlined into this only caller.)
| } | ||
| if res.MemorySwap != 0 { | ||
| switch { | ||
| case res.Memory == -1 || res.MemorySwap == -1: |
There was a problem hiding this comment.
(Turning this into if would be nicer.)
mtrmac
reviewed
Oct 31, 2025
| "strings" | ||
|
|
||
| "github.com/opencontainers/cgroups" | ||
| "github.com/opencontainers/cgroups/fs" |
There was a problem hiding this comment.
@lsm5 in all of these handler packages, we import cgroups/fs to declare fields which are never used, so it looks like this can be simplified further.
… and that makes #387 (comment) even more attractive.
mtrmac
reviewed
Oct 31, 2025
| } | ||
|
|
||
| type controllerHandler interface { | ||
| Create(*CgroupControl) (bool, error) |
There was a problem hiding this comment.
@lsm5 linuxCpusetHandler.Create still exists — what should happen there? Either it is all obsolete and the code should be removed, or it is still relevant and the .Create method (and .Destroy?) need to be reinstated.
lsm5
added a commit
to lsm5/container-libs
that referenced
this pull request
Nov 4, 2025
Ref: containers#417 (comment) Signed-off-by: Lokesh Mandvekar <[email protected]>
lsm5
added a commit
to lsm5/container-libs
that referenced
this pull request
Nov 4, 2025
Ref: containers#417 (comment) Signed-off-by: Lokesh Mandvekar <[email protected]>
lsm5
added a commit
to lsm5/container-libs
that referenced
this pull request
Nov 5, 2025
This broke a lot of stuff, so this should only be re-reverted once things are known to work in Podman and Buildah. Commits reverted: - e94388d - 26e4421 - b7be55c - cce5ec8 - 5f98dfb - ceaec36 - ed47881 - 2cf45c5 - 80309d7 - 8251431 - ff00d90 - 213e7ec Ref: containers#417 Signed-off-by: Lokesh Mandvekar <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following up on review comments in #387 . Reattempting with finer commits for easier review.